fix: replace abort() with cooperative wait in wait_for_run_task#576
Draft
xdustinface wants to merge 2 commits intorefactor/move-callbacks-to-cratesfrom
Draft
fix: replace abort() with cooperative wait in wait_for_run_task#576xdustinface wants to merge 2 commits intorefactor/move-callbacks-to-cratesfrom
abort() with cooperative wait in wait_for_run_task#576xdustinface wants to merge 2 commits intorefactor/move-callbacks-to-cratesfrom
Conversation
Introduce `EventHandler` trait in `dash-spv` with default no-op methods and make `DashSpvClient` generic over it (`H: EventHandler`, default `()`). The handler is passed at construction time and stored as `Arc<H>`: - `DashSpvClient::new()` takes `Arc<H>` and emits initial progress via `on_progress()` immediately after construction - `run()` subscribes to internal channels and dispatches events to the stored handler via monitoring tasks — no handler parameter needed - `impl EventHandler for ()` provides a zero-cost no-op default - `FFIEventCallbacks` implements `EventHandler` directly, eliminating the `FFIEventHandler` wrapper - FFI: callbacks pass to `dash_spv_ffi_client_new()`, `run()` is parameter-less - Add `TestEventHandler` that bridges events back to tokio channels for ergonomic `select!`-based integration tests - Add `LoggingEventHandler` to CLI binary
`abort()` can interrupt the cleanup sequence in `DashSpvClient::run()` (the `monitor_shutdown.cancel()` + `tokio::join!`), leaving monitor tasks running after FFI callback pointers are freed. Use cooperative wait with a timeout fallback instead.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## refactor/move-callbacks-to-crates #576 +/- ##
=====================================================================
+ Coverage 66.32% 66.51% +0.18%
=====================================================================
Files 311 312 +1
Lines 64976 65018 +42
=====================================================================
+ Hits 43097 43247 +150
+ Misses 21879 21771 -108
|
xdustinface
force-pushed
the
refactor/move-callbacks-to-crates
branch
from
aaddece to
23accc0
Compare
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
github-actions
bot
added
the
merge-conflict
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
abort()can interrupt the cleanup sequence inDashSpvClient::run()(themonitor_shutdown.cancel()+tokio::join!), leaving monitor tasks running after FFI callback pointers are freed. Use cooperative wait with a timeout fallback instead.Based on:
DashSpvClient#572